Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ref: Log for Swift #4201

Merged
merged 8 commits into from
Jul 30, 2024
Merged

ref: Log for Swift #4201

merged 8 commits into from
Jul 30, 2024

Conversation

brustolin
Copy link
Contributor

@brustolin brustolin commented Jul 29, 2024 •
edited
Loading

📜 Description

Standard log mechanism for swift.

💚 How did you test it?

Unit tests

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

#skip-changelog

@brustolin brustolin marked this pull request as ready for review July 29, 2024 11:40
@brustolin brustolin changed the title chore: Log in Swift chore: Log for Swift Jul 29, 2024
@codecov Codecov
Copy link

codecov bot commented Jul 29, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 90.16393% with 12 lines in your changes missing coverage. Please review.

Project coverage is 91.401%. Comparing base (fb2bfe8) to head (4a630a2).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4201       +/-   ##
=============================================
- Coverage   91.416%   91.401%   -0.016%     
=============================================
  Files          607       611        +4     
  Lines        48992     49011       +19     
  Branches     17669     17675        +6     
=============================================
+ Hits         44787     44797       +10     
- Misses        4112      4122       +10     
+ Partials        93        92        -1
Files Coverage Δ
SentryTestUtils/ClearTestState.swift 100.000% <ø> (ø)
SentryTestUtils/SentryLogExtensions.swift 100.000% <ø> (ø)
Sources/Sentry/SentryLevelHelper.m 100.000% <ø> (ø)
Sources/Sentry/SentryLevelMapper.m 100.000% <100.000%> (ø)
Sources/Sentry/include/SentryLog.h 90.000% <ø> (ø)
Sources/Swift/Helper/Log/SentryLevel.swift 100.000% <100.000%> (ø)
...ssionReplay/RRWeb/SentryRRWebBreadcrumbEvent.swift 100.000% <100.000%> (ø)
...ionReplay/SentrySRDefaultBreadcrumbConverter.swift 98.181% <100.000%> (ø)
Sources/Swift/Tools/SentryLogOutput.swift 100.000% <100.000%> (ø)
...ts/SentryTests/Helper/SentryFileManagerTests.swift 99.125% <100.000%> (ø)
... and 12 more

... and 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb2bfe8...4a630a2. Read the comment docs.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 29, 2024 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1223.17 ms 1241.07 ms 17.90 ms
Size 21.58 KiB 694.06 KiB 672.48 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d760c3f 1228.58 ms 1246.22 ms 17.64 ms
3f74b8f 1229.90 ms 1249.42 ms 19.52 ms
983de17 1260.57 ms 1263.68 ms 3.11 ms
5d6ce0e 1206.72 ms 1228.67 ms 21.95 ms
bd2afa6 1192.31 ms 1210.37 ms 18.05 ms
1e065bc 1239.69 ms 1258.18 ms 18.49 ms
3478fc5 1237.94 ms 1251.13 ms 13.19 ms
3297d6e 1196.08 ms 1205.43 ms 9.35 ms
5be7a7c 1223.25 ms 1237.12 ms 13.87 ms
a176fc4 1250.29 ms 1257.88 ms 7.59 ms

App size

Revision Plain With Sentry Diff
d760c3f 22.84 KiB 403.17 KiB 380.33 KiB
3f74b8f 21.58 KiB 418.78 KiB 397.20 KiB
983de17 22.84 KiB 403.19 KiB 380.34 KiB
5d6ce0e 22.85 KiB 405.38 KiB 382.53 KiB
bd2afa6 20.76 KiB 420.55 KiB 399.79 KiB
1e065bc 20.76 KiB 425.78 KiB 405.01 KiB
3478fc5 21.58 KiB 654.59 KiB 633.01 KiB
3297d6e 21.58 KiB 418.44 KiB 396.86 KiB
5be7a7c 21.58 KiB 671.89 KiB 650.31 KiB
a176fc4 22.84 KiB 403.24 KiB 380.39 KiB

philipphofmann
philipphofmann approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: I would prefix the PR title with ref cause it's a refactoring. I found a few minor issues to address. Thanks for doing this 🥇

LGTM.

Sources/Swift/Tools/SentryLog.swift Show resolved Hide resolved
Sources/Swift/Tools/SentryLogOutput.swift Outdated Show resolved Hide resolved
Tests/SentryTests/Integrations/SentryBaseIntegrationTests.swift Show resolved Hide resolved
Tests/SentryTests/SentryLevelTests.swift Show resolved Hide resolved
@brustolin brustolin changed the title chore: Log for Swift ref: Log for Swift Jul 30, 2024
@brustolin brustolin merged commit f64f819 into main Jul 30, 2024
62 of 67 checks passed
@brustolin brustolin deleted the feat/swift-lof branch July 30, 2024 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipphofmann philipphofmann philipphofmann approved these changes

@armcknight armcknight Awaiting requested review from armcknight armcknight is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@brustolin @philipphofmann @getsentry-bot